Conversation
| # list[uint8][4], float, int, uint32, uint8, etc. But more | ||
| # correctly, it should be exactly the dtype from the line above. | ||
| elt: Any | ||
| elt: list[int] | float |
There was a problem hiding this comment.
I don't think this accurately reflects the type intention of elt.
There was a problem hiding this comment.
elt is used in the file as
assert pixel[ix] == arr[y * img.width + x].as_py()[elt]
arr[(y * img.width + x) * elts_per_pixel + elt]
arr_pixel_tuple[elt]
pyarrow.array([elt] * (ct_pixels * elts_per_pixel), type=dtype)I acknowledge my understanding of this is not high, but I'm not seeing evidence that it's being used as anything more complex than initial data for a pyarrow array or for index access.
There was a problem hiding this comment.
As the comment says, it's a pixel or pixel component -- used for initialization of the data from the datashape tuple.
In the file from all of the cases of DataShape initialization, it's used as:
elt=[1, 2, 3, 4], # array of 4 uint8 per pixel
elt=3, # one uint8,
elt=0xABCDEF45, # one packed int, doesn't fit in a int32 > 0x80000000
elt=0x12CDEF45, # one packed int
3
1 << 24
3.14159
The comment listed is somewhat incorrect, as it's the python representation of whatever's in the dtype element. How to express that in a type system, I'm not sure.
I suspect that the types pass the checker because an int is a specific case of a float, but it's misleading.
There was a problem hiding this comment.
https://peps.python.org/pep-0484/
when an argument is annotated as having type float, an argument of type int is acceptable
There was a problem hiding this comment.
While it may be permissible, it doesn't clarify the intent of the code. The only point of a type annotation here is to help the next person to edit this, and if the type apparently conflicts with the usage of it it's going to raise more questions than it's going to answer.
|
Manually merged -- without the elt change. |
Suggestions for python-pillow#8908